-
Notifications
You must be signed in to change notification settings - Fork 369
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use default protocol/port when destination is service in antrea traceflow #6601
Conversation
If destination protocol/port isn't provided when destination is service in antrea traceflow, it just uses destinaton service's IP(clusterIP) and runs traceflow as Pod-to-IP(icmp) case which is incorrect. To fix this, we use the first protocol and port from the service's 'ports' list as default value. Fixes antrea-io#6594 Signed-off-by: Kumar Atish <[email protected]>
case corev1.ProtocolTCP: | ||
packet.IPProto = protocol.Type_TCP | ||
packet.TCPFlags = uint8(2) | ||
case corev1.ProtocolUDP: | ||
packet.IPProto = protocol.Type_UDP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The switch statement does not include a default case. While the current implementation handles TCP and UDP, if the service protocol is SCTP which is also currently not supported , we could add a default case !
default:
// Handle unsupported protocols, maybe log a warning
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defaults to ICMP which is consistent with other cases also.
In this case, the behaviour will be run traceflow as Pod-to-IP(Service's ClusterIP) with ICMP protocol.
/test-all |
If destination protocol/port isn't provided when destination is service in antrea traceflow, it just uses destinaton service's IP(clusterIP) and runs traceflow as Pod-to-IP(icmp) case which is incorrect.
To fix this, we use the first protocol/port from the service's 'ports' list as default value.
Fixes #6594